Using the variable on range scope tc in function literal (from scopelint)

Using the variable on range scope tc in function literal (from scopelint)

Not sure why the function was broken up this way? It's a bit harder to grok for me.

Supporting pagination for this one-off case can be achieved with minimal changes to QueryVotesByTxQuery. Let me know what you think.

func QueryVotesByTxQuery(cliCtx context.CLIContext, params types.QueryProposalParams) ([]byte, error) {
	events := []string{
		fmt.Sprintf("%s.%s='%s'", sdk.EventTypeMessage, sdk.AttributeKeyAction, types.TypeMsgVote),
		fmt.Sprintf("%s.%s='%s'", types.EventTypeProposalVote, types.AttributeKeyProposalID, []byte(fmt.Sprintf("%d", params.ProposalID))),
	}

	var (
		votes        []types.Vote
		searchResult *sdk.SearchTxsResult
		err          error
	)

	page := 1

	searchResult, err = utils.QueryTxsByEvents(cliCtx, events, page, 100)
	if err != nil {
		return nil, err
	}

	// Search for votes until the tx results are exhausted or we've reached the
	// pagination limit.
	for searchResult.Count != 0 || len(votes) < params.Limit {
		for _, info := range searchResult.Txs {
			for _, msg := range info.Tx.GetMsgs() {
				if msg.Type() == types.TypeMsgVote {
					voteMsg := msg.(types.MsgVote)

					votes = append(votes, types.Vote{
						Voter:      voteMsg.Voter,
						ProposalID: params.ProposalID,
						Option:     voteMsg.Option,
					})
				}
			}
		}

		page++

		searchResult, err = utils.QueryTxsByEvents(cliCtx, events, page, 100)
		if err != nil {
			return nil, err
		}
	}

	start, end := client.Paginate(len(votes), params.Page, params.Limit, 100)
	if start < 0 || end < 0 {
		votes = []types.Vote{}
	} else {
		votes = votes[start:end]
	}

	if cliCtx.Indent {
		return cliCtx.Codec.MarshalJSONIndent(votes, "", "  ")
	}

	return cliCtx.Codec.MarshalJSON(votes)
}

i splitted whole function so that i can actually test pagination logic (see query_test.go).
otherwise, body of the function is similar to yours, with one distinction - if the previous tx query returned less then full chunk we already know that tx results are exhaused and query terminated earlier

Let's keep the logic in QueryVotesByTxQuery because unit testing private functions is not always an option -- the end result is the same, you're still unit testing the same business logic.

You can update the above snippet to handle the case I missed 👍

in order to unit test QueryVotesByTxQuery i need to mock calls to SearchTx, i do it with passing custom querier function. am i missing something that can be used for this purpose?

Yes, you're right! We can add the function as a parameter to QueryVotesByTxQuery?

imo bad api. but not worth arguing, if you feel that this is ok i will pass as param

Thanks @dshulyak!

I would remove this method and move the pagination logic within the querier handler like other modules do.

i checked proposals method when i was adding this one - https://github.com/cosmos/cosmos-sdk/blob/master/x/gov/keeper/proposal.go#L114-L148

I'm not sure I follow? This method is not needed. Pagination can simply be done in queryVotes.

i mean that when i was adding this method i intentionally made it consistent with that one (e.g. kept pagination in keeper)

I see. Yeah, but this isn't necessary in this case.

i checked proposals method when i was adding this one - https://github.com/cosmos/cosmos-sdk/blob/master/x/gov/keeper/proposal.go#L114-L148

i splitted whole function so that i can actually test pagination logic (see query_test.go).
otherwise, body of the function is similar to yours, with one distinction - if the previous tx query returned less then full chunk we already know that tx results are exhaused and query terminated earlier